Skip to content

Fix pipe-stage ?, impl Trait for Type, trait headers, dyn type args#70

Merged
StreamDemon merged 2 commits into
fix/parser-correctness-basefrom
fix/pipe-stage-impl-trait-dyn
Jul 2, 2026
Merged

Fix pipe-stage ?, impl Trait for Type, trait headers, dyn type args#70
StreamDemon merged 2 commits into
fix/parser-correctness-basefrom
fix/pipe-stage-impl-trait-dyn

Conversation

@StreamDemon

@StreamDemon StreamDemon commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fixes the P1 pipe-stage ? misparse: x |> f? parsed as x |> (f?) because |> was an ordinary Pratt binary operator and the ? postfix bound inside the right operand. §5.7 and §16 (pipe_stage) mandate (x |> f)? — a stage's trailing ? wraps the accumulated pipe application. The RHS of |> now goes through a dedicated pipe_stage() production (stage_callee [ "(" args ")" ]: path, optional turbofish, .field chains) and the caller wraps the accumulated pipe in ErrorProp. Closure stages (x |> (|v| ...)) and turbofish on non-final stage segments error as not-yet-implemented.
  • Fixes impl Trait for Type (previously "expected item" — for was never handled): impl_block() now records Option<TraitRef> in the AST and parses where clauses.
  • Fixes trait headers: trait Convert<T>, trait Loggable: Printable, and where clauses now parse; supertrait bounds are parsed structurally (trait_ref | lifetime, +-separated) for diagnostics but not stored, consistent with the documented bootstrap posture.
  • Fixes dyn Trait<Args>: Type::Dyn now carries a TraitRef { path, args } (grammar's trait_ref = type_path [ type_args ]), so &dyn Iter<Item = i64> and Box<dyn Iter<Item = i64>> parse.

Grammar-correct behavior changes (regression-guarded by tests): x |> a + b is now (x |> a) + b — a stage is only a callee, so + b applies to the finished pipe; struct literals are no longer valid pipe stages.

Stacked-PR note: this PR targets the integration branch fix/parser-correctness-base (PR #69), not main. Wave-level docs reconciliation happens on the base PR at merge time; this PR updates only the crates/AGENTS.md subset-contract lines its fixes change.

Spec follow-up notes (no spec edits here; candidates for the open spec-debt bucket, cf. #54):

  1. docs/reference/operator-precedence.md lists |> at precedence 8 but doesn't state that the pipe RHS is a pipe_stage; a naive table reading suggests x |> a + b = x |> (a + b), while §16 forces (x |> a) + b. Worth an explicit sentence in the "Key Interaction" section.
  2. §16 stage_callee grammatically permits turbofish on non-final segments (a.b::<T>.c) but §5.6/§5.7 define no semantics for it; the parser reports it as not-yet-implemented.
  3. §16 path_expr is ("Self" | IDENT) { "::" IDENT } (no bare self), yet x |> self.helper is a natural spelling inside actor methods; the bootstrap path() accepts self leniently (pre-existing behavior, unchanged here).

Related Issue

None (review-driven; part of the parser correctness wave tracked in PR #69).

Spec Sections Affected

None changed — this PR implements §16 pipe_stage/trait_ref/impl_block/trait_def as written. See the spec follow-up notes above.

Checklist

  • Code follows the Sploosh design principles (one way to do it, explicit over implicit, etc.)
  • Documentation updated in relevant docs/ pages (crates/AGENTS.md subset contract)
  • Tests added or updated
  • All build targets still compile (if applicable)
  • Spec-only PR (skip Build Targets section if checked)

Build Targets Tested

  • N/A (docs/spec only) — no codegen targets exist yet; "build" = the Rust workspace.

Test Plan

  • 15 new unit tests in sploosh-parser, including AST-shape assertions: pipe_stage_question_wraps_accumulated_pipe asserts the exact ErrorProp(Binary("|>", input, parse)) tree; chained a |> f? |> g? nesting; method-chain, call-args, and turbofish stages; (x |> a) + b regression guard; error-message tests for x |> 42 and x |> (v); impl/trait/dyn shape assertions.
  • 2 new corpus fixtures: tests/corpus/traits_impls.sp (trait generics/supertraits/where, inherent + trait impls, Box<dyn Iter<Item = i64>>, &dyn Convert<i64>) and tests/corpus/pipes.sp (every accepted stage shape).
  • cargo fmt --all -- --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace (32 tests) all green locally and via scripts/docker-check.ps1 -Build (Ubuntu parity).

Summary by cubic

Fixes pipe-stage parsing so x |> f? becomes (x |> f)? and adds support for impl Trait for Type, richer trait headers, and dyn Trait<Args> types.

  • Bug Fixes

    • |> now parses the RHS as a pipe_stage; the trailing ? wraps the accumulated pipe. Non-callee stages are rejected; closure stages and non-final turbofish are rejected with explicit not-yet-implemented errors.
    • impl Trait for Type parses correctly; the AST records trait_ref and supports where clauses.
    • trait headers accept generics, supertrait bounds, and where clauses (bounds parsed for validation, not stored).
    • dyn Trait<Args> stores a full trait reference, enabling cases like &dyn Iter<Item = i64>.
  • Migration

    • x |> a + b is now parsed as (x |> a) + b. Add parens if you relied on the old grouping.
    • Struct literals and other non-callee expressions are no longer valid pipe stages. Use a function or method stage instead.

Written for commit 1c100cb. Summary will update on new commits.

Review in cubic

Four parser bugs found by an empirical review against spec section 16,
all in surface the subset contract claims to support:

- `x |> f?` parsed as `x |> (f?)` because `|>` was an ordinary binary
  operator and the `?` postfix bound inside the right operand. Spec
  sections 5.7 and 16 (pipe_stage) mandate `(x |> f)?` -- the stage's
  trailing `?` wraps the accumulated pipe application. The RHS of `|>`
  now goes through a dedicated pipe_stage() production (callee path,
  optional turbofish, field chain, optional args) and the caller wraps
  the accumulated pipe in ErrorProp. Closure stages and non-final-
  segment turbofish remain not-yet-implemented with explicit errors.
- `impl Trait for Type` failed with "expected item": impl_block() never
  handled `for`. It now records an optional TraitRef and parses `where`
  clauses.
- `trait Convert<T>` and `trait Loggable: Printable` failed: trait_def()
  skipped straight from the name to the body. Generic params, supertrait
  bounds (parsed structurally, not stored), and `where` clauses now
  parse.
- `&dyn Iter<Item = i64>` failed: the dyn branch of ty() ignored
  type_args. Type::Dyn now carries a TraitRef (grammar's trait_ref =
  type_path [type_args]).

Grammar-correct behavior changes, regression-guarded by tests:
`x |> a + b` is now `(x |> a) + b` (a stage is only a callee), and
struct literals are no longer valid pipe stages.

Adds 15 unit tests (AST-shape assertions for every pipe edge case plus
impl/trait/dyn shapes), two corpus fixtures (traits_impls.sp, pipes.sp),
and updates the crates/AGENTS.md subset contract for the lines these
fixes change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H3CinyyqWL6SfCLrKGm3ja

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 6 files

Confidence score: 5/5

  • In crates/AGENTS.md, the new “closure pipe stages ... and turbofish ...” entry is a sentence fragment, which risks minor confusion/inconsistency in the not-yet-implemented list but not runtime behavior; this PR is safe to merge, and you can de-risk fully by rewriting that line to include a clear verb/predicate before or right after merge.
Architecture diagram
sequenceDiagram
    participant Src as Source Code
    participant Lex as Lexer
    participant Pars as Parser
    participant Pipe as pipe_stage()
    participant TraitRef as trait_ref()
    participant Ast as AST Output

    Note over Src,Ast: Parsing entry

    Src->>Lex: read source text
    Lex-->>Pars: token stream

    Note over Pars: Expression parsing for `|>` operator

    Pars->>Pars: expr(min_prec) – go to `|>` handling
    alt Token equals "|>"
        Pars->>Pipe: pipe_stage()
        Note over Pipe: Parse §16 stage_callee<br/>(path, turbofish, field chain,<br/>optional (args))
        Pipe-->>Pars: stage expression
        alt Next token is "?"
            Pars->>Pars: compute ErrorProp(Binary(|>, lhs, stage))
        end
        Pars-->>Ast: pipe expression tree
    else Other operators
        Pars->>Pars: continue normal precedence climb
    end

    Note over Pars: Item parsing for `impl Trait for Type`

    Src->>Lex: impl User {} or impl Printable for User
    Lex-->>Pars: tokens starting with "impl"
    Pars->>Pars: impl_block()
    Pars->>Pars: parse first type (User)
    alt Next token is "for"
        Pars->>TraitRef: trait_ref()
        TraitRef->>TraitRef: parse path + type_args
        TraitRef-->>Pars: TraitRef struct
        Pars->>Pars: parse target type (User)
        Pars->>Pars: ImplBlock with trait_ref = Some
    else No "for"
        Pars->>Pars: ImplBlock with trait_ref = None (inherent impl)
    end
    Pars->>Pars: maybe_where_clause(), skip body
    Pars-->>Ast: ImplBlock node

    Note over Pars: Trait definition with supertrait bounds

    Src->>Lex: trait Loggable: Printable { }
    Lex-->>Pars: tokens starting with "trait"
    Pars->>Pars: trait_def()
    Pars->>Pars: parse name, maybe_generic_params()
    Pars->>Pars: eat(Colon)?
    alt Colon present
        Pars->>Pars: bounds()
        loop for each bound
            Pars->>TraitRef: trait_ref() (or lifetime)
            TraitRef-->>Pars: trait_ref node (validated, not stored)
        end
    end
    Pars->>Pars: maybe_where_clause(), skip body
    Pars-->>Ast: Trait node (bounds discarded)

    Note over Pars: Type parsing for `dyn Trait<Args>`

    Src->>Lex: &dyn Iter<Item = i64>
    Lex-->>Pars: tokens including "dyn"
    Pars->>Pars: ty()
    alt Token is "dyn"
        Pars->>TraitRef: trait_ref()
        TraitRef->>TraitRef: parse path + type_args (e.g., Iter<Item = i64>)
        TraitRef-->>Pars: TraitRef { path, args }
        Pars->>Pars: create Type::Dyn(trait_ref)
        Pars-->>Ast: Dyn type node
    else Other types
        Pars->>Pars: parse as before
    end
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/AGENTS.md
Cubic review on PR #70: the new list entry was the only clause in the
not-yet-implemented list without a verb. It now states what the parser
actually does — rejects both shapes with explicit not-yet-implemented
parse errors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H3CinyyqWL6SfCLrKGm3ja

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Requires human review: Parser changes to core grammar (pipe stages, trait impls, dyn with args) with breaking semantics for |>. High risk of breakage; requires human review.

Re-trigger cubic

@StreamDemon StreamDemon marked this pull request as ready for review July 2, 2026 07:39
@StreamDemon StreamDemon merged commit d6bd425 into fix/parser-correctness-base Jul 2, 2026
2 checks passed
@StreamDemon StreamDemon deleted the fix/pipe-stage-impl-trait-dyn branch July 2, 2026 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant